[CMake] Move headers to include/LLVMSPIRVLib to match install layout#3581
Open
wenju-he wants to merge 7 commits intoKhronosGroup:mainfrom
Open
[CMake] Move headers to include/LLVMSPIRVLib to match install layout#3581wenju-he wants to merge 7 commits intoKhronosGroup:mainfrom
wenju-he wants to merge 7 commits intoKhronosGroup:mainfrom
Conversation
Currently, headers are only available in their source locations during
build time, but are installed to a flattened 'LLVMSPIRVLib' directory.
This creates a discrepancy between in-tree builds (which must use
raw source paths) and out-of-tree builds (which use the install path).
This commit copies headers to '${CMAKE_BINARY_DIR}/include/LLVMSPIRVLib'
, allowing downstream projects (e.g. https://github.com/intel/opencl-clang)
to use '#include <LLVMSPIRVLib/header.h>' consistently in all scenarios.
Changes:
- Copy headers to build directory using copy_if_different to preserve
timestamps and avoid unnecessary downstream rebuilds
- Update all internal includes to use 'LLVMSPIRVLib/' prefix to match
the installed layout
- Add PUBLIC include directories with BUILD_INTERFACE and INSTALL_INTERFACE
generator expressions for automatic include path propagation
This pattern is common across LLVM projects (Clang lib/Headers, LLDB,
libcxx) to enable in-tree builds to mimic installed layouts without
requiring actual installation.
Member
|
Retriggering CI. |
Instead of copying headers to the build directory at build time, move them directly to include/LLVMSPIRVLib/ in the source tree. This is a simpler and more standard approach for C++ library organization. This approach provides: - Simpler CMake configuration (22 lines removed) - No build-time copying overhead - Single source of truth for headers - Standard library layout matching the installed structure - Easier maintenance and IDE navigation The headers are now located at: - include/LLVMSPIRVLib/LLVMSPIRVLib.h - include/LLVMSPIRVLib/LLVMSPIRVOpts.h - include/LLVMSPIRVLib/LLVMSPIRVExtensions.inc All internal includes already use the LLVMSPIRVLib/ prefix from the previous commit, so this change completes the restructuring. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The llvm-spirv tool links to LLVMSPIRVLib, which now declares its include directories as PUBLIC. CMake's target propagation automatically adds these include paths to any target that links to the library. Explicitly adding LLVM_SPIRV_INCLUDE_DIRS to llvm-spirv's include directories is therefore redundant and can be removed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ories The INSTALL_INTERFACE generator expression only takes effect when the target is exported via CMake config files (LLVMSPIRVLibConfig.cmake). Since SPIRV-LLVM-Translator does not currently generate or install CMake config files, this expression has no effect. The BUILD_INTERFACE is retained as it correctly provides include paths for in-tree builds where targets link to LLVMSPIRVLib. This can be re-added in the future when CMake package config file export is implemented. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
wenju-he
changed the title
wenju-he
changed the title
Revert the BUILD_INTERFACE related changes from this branch to split
them into a separate PR. The BUILD_INTERFACE fix is independent of the
header reorganization and should be reviewed separately.
Changes:
- Revert lib/SPIRV/CMakeLists.txt: target_include_directories back to
PRIVATE with direct ${LLVM_SPIRV_INCLUDE_DIRS} inclusion
- Restore tools/llvm-spirv/CMakeLists.txt: Add back explicit
${LLVM_SPIRV_INCLUDE_DIRS} since it's no longer inherited from
LLVMSPIRVLib
The BUILD_INTERFACE fix will be submitted as a standalone PR in the
build-interface-fix branch.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, headers are only available in their source locations during
build time, but are installed to a flattened 'LLVMSPIRVLib' directory.
This creates a discrepancy between in-tree builds (#include
"LLVMSPIRVOpts.h") and out-of-tree builds (#include
"LLVMSPIRVLib/LLVMSPIRVOpts.h") for downstream projects.
This PR restructures the headers by moving them directly to
include/LLVMSPIRVLib/ in the source tree, allowing downstream
projects (e.g., https://github.com/intel/opencl-clang) to use
#include <LLVMSPIRVLib/header.h> consistently in all scenarios.
Organizing headers in subdirectories matching the installed layout is
a common pattern across LLVM projects (e.g., Clang lib/Headers, LLDB, libcxx).